Skip to content

New lint: unused_enumerate_value #14443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

profetia
Copy link
Contributor

for (i, _) in iter.enumerate() can be replaced with for i in 0..iter.len() if the iterator implements ExactSizeIterator.

Closes #14430

changelog: [unused_enumerate_value]: new lint

@rustbot
Copy link
Collaborator

rustbot commented Mar 20, 2025

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 20, 2025
@profetia profetia force-pushed the issue14430 branch 2 times, most recently from f99f935 to 5428c6b Compare March 20, 2025 05:43
@blyxyas
Copy link
Member

blyxyas commented Mar 23, 2025

Ideally we should also check if the arbitrary type implements .len() as a method, wouldn't we? Search, while not the best regex for looking this up, we can see that it's a very common pattern to implement .len() directly on the struct, without implementing the trait.

impl ExactSizeIterator, the trait isn't very well known.

@profetia
Copy link
Contributor Author

profetia commented Mar 23, 2025

Emm.. I doubt it because len does not belongs to any trait in the std, so the len implemented by a user-defined type may not has the sematics of what we intended here.

@blyxyas
Copy link
Member

blyxyas commented Mar 23, 2025

In that case, we could just emit a warning if it's not coming from a ExactSizeIterator. I don't think we'll get much use if we limit the lint to just the trait.

@profetia profetia force-pushed the issue14430 branch 2 times, most recently from 79ce2bf to 81c7daa Compare March 24, 2025 09:41
@profetia
Copy link
Contributor Author

Updated. Now arbitrary type is covered.

@Snowiiii
Copy link

@blyxyas does this look good now?

@rustbot

This comment has been minimized.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 31, 2025
Comment on lines +24 to +25
&& let Some((DefKind::AssocFn, call_id)) = cx.typeck_results().type_dependent_def(arg.hir_id)
&& cx.tcx.is_diagnostic_item(sym::enumerate_method, call_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Smart here to check for both Enumerate type and .enumerate(). This would avoid us linting on cases like iter.enumerate().map(), which could use iter in unforeseen ways.

&& let receiver_ty = cx.typeck_results().expr_ty(recv)
// TODO: Replace with `sym` when it's available
&& let Some(exact_size_iter) = get_trait_def_id(cx.tcx, &paths::ITER_EXACT_SIZE_ITERATOR)
&& implements_trait(cx, receiver_ty, exact_size_iter, &[])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this as an open question for the FCP, but I still think that we should not only lint for types that implement iter_exact_size_iterator, as len() being an idiomatic standard is practically guaranteed. In fact, len() that does not report exact length as an usize would be considered a very bad practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking for len is implemented in remove_trailing_iter because we should only remove the iter call if the iterator is created in this expr.

@blyxyas
Copy link
Member

blyxyas commented Apr 6, 2025

Everything is looking great! I opened the FCP on Zulip

@profetia
Copy link
Contributor Author

From the FCP I find it not that reasonable as I initial thought about this lint. I'll close this pr for now.

@profetia profetia closed this Apr 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unneded iter for loop
4 participants